Skip to content

feat(ui,client): version navigation and non-destructive prompt rewrite - #517

Open
Zerlight wants to merge 23 commits into
ruocheng/code-637from
ruocheng/code-638
Open

Zerlight wants to merge 23 commits into
ruocheng/code-637from
ruocheng/code-638

Conversation

@Zerlight

@Zerlight Zerlight commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

Phase 4 of CODE-627 — Conversation turn graph & immutable attachment store. Linear: https://linear.app/arcbox/issue/CODE-638/featuiclient-version-navigation-and-non-destructive-prompt-rewrite

Stack: #516this PR (ruocheng/code-638, base ruocheng/code-637) ← top of the stack. Merge bottom-up; this PR's diff is only its own commits.

Phase 4: ‹ 1/N › version navigation from the graph's sibling ordinals, a client-local parked view (pure reads that never move the host default), and the non-destructive rewrite — editing prompt T submits a sibling under parent(T) through turn.submit, sends from a parked version continue from its last completed turn, and failed or cancelled turns keep their ordinal and badge. Engine side: inactive lineages read their own history, a failed leaf's shared prefix reads from the live history, every settle re-announces the tree at the same revision, and a relaunch whose dispatch failed is unwound so the thread keeps its own history.

This PR also carries the review-round fixes for the whole stack (the code they touch moved in this branch, and the stack merges as a unit): failed-turn attribution, the delete-time launch guard, the preceding-checkpoint binding, the upload cap and idle reap, the read-walk identity check, the inline image count, the readonly_file conversion, and the legacy upload frame's wire shape. Each is recorded on its own PR's thread and on CODE-627.

Commits

  • feat(engine): attribute an inactive lineage against its own run history on read
  • feat(client-core): explicit-parent turn.submit, leaf-targeted seeds, and frozen parked stores
  • feat(ui): version navigation, turn state badges, and the parked-lineage notice
  • feat(workbench): sibling turns, explicit-parent submits, and leaf reads in the dev mock
  • feat(workbench): browse turn versions and rewrite prompts as siblings through the turn graph
  • fix(engine,workbench): announce a failed sibling as a graph shape change and refresh the tree on park
  • fix(engine,schema): unwind a failed relaunch and decide root edits from the session's first root
  • fix(engine,client-core): read a failed leaf's shared prefix, announce settles, and hide unrun turns
  • fix(workbench,ui): follow the host default after a submit, freeze only parked reads, and continue from the last completed turn
  • fix(workbench): mirror the daemon's settle announcements, pre-dispatch refusals, cancelled settles, and admit order in the dev mock
  • fix(engine): attribute a failed turn's provider rows and align the lineage around it
  • fix(engine,workbench): guard launches mid-delete, replay responding asks, pin read cursors to the item count, and match the mock's command text
  • fix(engine): file a preceding checkpoint under the parent's own run
  • test(client-core): pin the fork re-read against the graph-move shortcut
  • fix(engine): bound live uploads, reap idle stages, and refuse a replayed begin with other fields
  • fix(client-core,workbench): pin read pages to the first blob and mirror the begin replay rule in the mock
  • fix(engine): count inline images against the harness cap and hand readonly_file projections to adapters as file links
  • fix(schema,engine): keep the legacy upload frame open on the wire and cap its name in the handler
  • fix(client-core,schema): refuse an over-long attachment name or MIME type before the begin frame
  • fix(workbench): mint no preview URL after a session switch, group siblings once, and forget the mock's begin replay on commit
  • fix(engine): read another version without the active run's live tail
  • fix(engine): read every turn from the history its run wrote, never a fork's copy of it
  • fix(client-core): keep a parked store's session state live so the composer never falls back to defaults

Verification

Every commit passed pnpm check:ci and pnpm test at its own tip; the stack tip (76e383bf) is at pnpm check:ci 0 errors, pnpm test 3428 passed / 1 skipped. Adversarial reviewers (one per axis, isolated read-only worktrees) reviewed the branch; each P1/P2 was reproduced with a failing test or a probe step before its fix — the round-by-round record, including the six bot review rounds on #507#517, is in the Linear issue's comments. Headless-Chrome probe of dev:mock: two prompts → edit → 2/2 (zero stale alerts during the edit) → 1/2 + parked notice → Back to latest → fail edit 3/3 Failedrefuse edit 3/4 Failed 4/4 Failed → send → 5/5 under the first turn; no console errors. The real-daemon edit path was then verified against live Claude Code (dev daemon, headless Chrome screenshots on CODE-627): it found three bugs the mock cannot show — a parked read carried the active run's live tail, the shared prefix rendered from the fork's re-stamped copy ("Thought for 10 seconds" vs "1 second"), and the parked composer fell back to defaults — fixed in the last three commits, after which both versions render their shared turns identically and the composer keeps policy, model, and effort while parked.

Checklist

  • pnpm check:ci and pnpm test both pass (no Rust changes)
  • I ran the affected surface and observed the change working — the compiled renderer against dev:mock in headless Chrome
  • Wire: bumped 80 → 81 (additive SessionRun.abandonedAt); resource.source.upload keeps its v79 shape
  • New code and assets are my own work
  • Docs and comments are updated where behavior changed (AGENTS.md and module docs in this branch)

@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

@linear-code

linear-code Bot commented Sep 7, 2026

Copy link
Copy Markdown

CODE-638

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds conversation-version navigation and non-destructive prompt rewriting across the engine, client, workbench, and presentation layers.

  • Introduces explicit-parent submissions with graph-revision conflict detection.
  • Supports leaf-targeted reads and frozen stores for parked lineages.
  • Adds sibling navigation, lineage notices, and terminal-state badges.
  • Updates engine projection, settlement announcements, failed-turn handling, and relaunch cleanup.
  • Brings the development mock and integration tests in line with daemon behavior.

Confidence Score: 5/5

The PR appears safe to merge; no actionable new failure or outstanding repository-rule violation was identified.

The follow-up changes correctly keep live error-reporting metadata transient while preserving deterministic operation replay, and they reject cross-session operation-ID reuse without affecting correctly generated submissions.

Important Files Changed

Filename Overview
packages/host/engine/src/conversation/turn-service.ts Persists and settles graph turns, announces shape and terminal-state changes, and keeps live-only failure-reporting metadata out of replayed records.
packages/host/engine/src/session/lifecycle-service.ts Adds explicit-parent submission admission, cross-session operation ownership checks, and failure cleanup for launch and dispatch paths.
packages/client/core/src/conversation-store.ts Adds frozen projection stores so parked lineage reads do not consume events from the active run.
packages/client/workbench/src/surface/lineage.ts Implements lineage traversal, sibling version selection, active-lineage detection, and safe continuation-parent selection.
packages/client/workbench/src/surface/workbench.tsx Integrates parked-version navigation, explicit-parent rewrites and continuations, and return-to-default behavior.
packages/presentation/ui/src/chat/turn-version-nav.tsx Adds the user-facing version navigation controls and turn-state presentation.

Sequence Diagram

sequenceDiagram
  participant UI as Workbench UI
  participant Client as Client Core
  participant Engine as Host Engine
  participant Store as Conversation Store

  UI->>Client: Read selected leafTurnId
  Client->>Engine: conversation.read(leafTurnId)
  Engine->>Store: Load root-to-leaf projection
  Store-->>Engine: Lineage events and graph revision
  Engine-->>Client: Projection seed
  Client-->>UI: Live store or frozen parked store

  alt Rewrite an existing prompt
    UI->>Client: submitTurn(input, parent, revision)
    Client->>Engine: turn.submit with explicit parent
    Engine->>Store: Validate revision and persist sibling
    Engine-->>Client: turn.submitted
    Engine-->>UI: conversation.graph.changed
  else Continue from a parked version
    UI->>Client: submitTurn(input, last completed turn, revision)
    Client->>Engine: turn.submit with explicit parent
    Engine->>Store: Persist child and move default leaf
    Engine-->>UI: conversation.graph.changed
  end
Loading

Reviews (3): Last reviewed commit: "fix(workbench): mirror the daemon's sett..." | Re-trigger Greptile

@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Your Claude subscription has reached its daily usage limit, blocking all Pullfrog runs until it resets.

Every Pullfrog run on arcboxlabs has failed since September 7 (5 runs, no successes), so this review did not happen.

Anthropic's Claude subscription has hit its daily usage limit, which resets at 11:30am UTC. Since no ANTHROPIC_API_KEY is stored as a fallback, every run that tries to use the Claude model fails when the subscription window is exhausted.

To fix it:

  1. Add an ANTHROPIC_API_KEY to your repo's GitHub Actions secrets — Pullfrog will use it automatically when the subscription hits its limit, keeping runs working.
  2. If you prefer to stay subscription-only, wait until 11:30am UTC for the limit to reset and re-trigger any failed runs.

Copilot AI lite review requested due to automatic review settings September 7, 2026 07:25
@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings September 7, 2026 08:24
@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings September 7, 2026 10:21
@pullfrog

pullfrog Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11:30am (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings September 7, 2026 13:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new graph-based prompt rewrite path still blocks (and strips) stored attachment refs in UserMessage, preventing non-destructive rewrites from preserving attachments as intended.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/presentation/ui/src/chat/user-message.tsx:120

  • When submitting an edit, stored attachment blocks are always stripped from retainedBlocks, which would drop attachment refs on rewrite. With graph rewrites enabled, these resource_link attachment blocks should be preserved so promptBlocksFromComposer can convert them into attachment_ref blocks on submit.
    packages/presentation/ui/src/chat/user-message.tsx:93
  • Edits are blocked whenever the prompt contains stored attachments (resource_linkattachmentIdFromUri), but graph rewrites can submit attachment_ref blocks (via promptBlocksFromComposer) and should be allowed to rewrite prompts that include stored attachment refs. As-is, the UI shows editAttachmentsUnsupported even when rewritesViaGraph is enabled and the message is a known graph node.

This issue also appears on line 115 of the same file.

  • Files reviewed: 56/56 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/client/workbench/src/surface/lineage.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes — incremental re-review of the two commits since 4952b99:

  • fdda262 fix(engine)upload-service.ts gains MAX_LIVE_UPLOADS = 32 with a reserved counter released via Effect.ensuring, a UPLOAD_IDLE_MS = 5 min idle reap that runs on the next begin, and a sameDeclaration check that refuses a replayed operationId carrying different declared fields instead of handing back another upload's id.
  • 8278289 fix(client-core,workbench) — the attachment read walk now pins every page to the first page's blobId/sizeBytes (plus offset, overrun, and empty-slice guards), and dev-mock-host.ts mirrors the daemon's begin-replay field check.

The bounding logic holds up. Effect.ensuring fires on success, failure, defect, and interruption, and the daemon forks each request through FiberSet.runtime, so exactly one reserve() pairs with one release() per admitted begin and hands off cleanly to live.size. sameDeclaration cannot false-refuse a legitimate replay either — attachment-store.beginUpload returns the caller's lease object verbatim, so name/mimeType round-trip unchanged. The new tests genuinely fail without their fixes: the cap test asserts exactly one refusal out of MAX_LIVE_UPLOADS + 1 concurrent begins (it would see zero without reserve()), and the reap test pins the exact UPLOAD_IDLE_MS - 1 boundary.

ℹ️ Stack sequencing

Both new commits fix the attachment-upload subsystem, which belongs to the PR below this one in the stack — the base here is ruocheng/code-637, not master. Merging bottom-up ships that PR with no live-upload cap, no idle stage reap, and an unpinned read walk, which are the exact surfaces these commits close. Worth confirming this is deliberate rather than a mis-targeted branch.

Technical details

Files touched by the two new commits and the layer that owns them:

  • packages/host/engine/src/attachment/upload-service.ts — attachment upload store (PR below)
  • packages/client/core/src/client/attachment-channel.ts — attachment read channel (PR below)
  • packages/client/workbench/src/mock/dev-mock-host.ts — mock parity for the above

Secondary: the PR body lists 10 commits against a commitCount of 16, and describes only version navigation / prompt rewrite. A reader landing here would not expect upload-bounding changes; refreshing the body would help whoever reviews the merge.

ℹ️ Nitpicks

  • packages/foundation/schema/src/wire/attachment.ts:36 — the doc comment "a second begin with the same id returns the first" is now an understatement. After fdda262 that holds only when the declared fields match; otherwise the begin is refused with invalid_request. Not in this diff, so no inline anchor.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/client/workbench/src/mock/dev-mock-host.ts
Copilot AI review requested due to automatic review settings September 7, 2026 13:41
…y parked reads, and continue from the last completed turn
…h refusals, cancelled settles, and admit order in the dev mock
…sks, pin read cursors to the item count, and match the mock's command text
…donly_file projections to adapters as file links
…lings once, and forget the mock's begin replay on commit
Copilot AI review requested due to automatic review settings September 14, 2026 11:37
@pullfrog

pullfrog Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Your organization has disabled Claude subscription access for Claude Code. Ask your Claude organization's admin to re-enable it in the Claude Console, or set an ANTHROPIC_API_KEY for this repo instead, then re-trigger the run.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The changes require final human review because they are too complex or risky for automated approval.

Review details
  • Files reviewed: 67/67 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@pullfrog

pullfrog Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Your Claude organization has disabled subscription access for Claude Code, so every run on the linkcode repo fails.

Every Pullfrog run on arcboxlabs has failed since September 14 (5 runs, no successes), so this review did not happen.

Every run on the linkcode repo fails with "Your organization has disabled Claude subscription access for Claude Code." The repo is pinned to anthropic/claude-opus, which uses a Claude Pro/Max subscription, but the account's Claude organization has turned off subscription access for Claude Code.

To fix it:

  1. Ask your Claude organization's admin to re-enable subscription access for Claude Code in the Claude Console.
  2. If that is not possible, add an ANTHROPIC_API_KEY in your repo consolethe setup docs cover how to get one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants